enhance(test-benchmark): use config file for fixed opcode count scenarios#1790
Conversation
LouisTsai-Csie
left a comment
There was a problem hiding this comment.
Thanks a lot for this! I left some suggestions, but I’m happy to discuss further. I’ll share this with Kamil to confirm it aligns with their needs.
packages/testing/src/execution_testing/cli/pytest_commands/plugins/shared/benchmarking.py
Outdated
Show resolved
Hide resolved
7ca99dc to
7c68d19
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/osaka #1790 +/- ##
===============================================
- Coverage 87.31% 83.87% -3.45%
===============================================
Files 541 402 -139
Lines 32832 25101 -7731
Branches 3015 2285 -730
===============================================
- Hits 28668 21053 -7615
- Misses 3557 3609 +52
+ Partials 607 439 -168
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
LouisTsai-Csie
left a comment
There was a problem hiding this comment.
Add some suggestion for the parser! Thanks
packages/testing/src/execution_testing/cli/pytest_commands/plugins/shared/benchmarking.py
Outdated
Show resolved
Hide resolved
5506d25 to
14b6b64
Compare
14b6b64 to
697a7d0
Compare
…d gas bench values
There was a problem hiding this comment.
Thanks!! Only a small logic adjustment is needed.
Here is an example using this test:
@pytest.mark.repricing(contract_balance=0)
@pytest.mark.parametrize("contract_balance", [0, 1])
def test_selfbalance(
benchmark_test: BenchmarkTestFiller,
contract_balance: int,
) -> None:
"""Benchmark SELFBALANCE instruction."""
benchmark_test(
code_generator=ExtCallGenerator(
attack_block=Op.SELFBALANCE,
contract_balance=contract_balance,
),
)When running in pure --fixed-opcode-count or --gas-benchmark-values mode, both parameters should be executed (contract_balance = 0 and 1).
This results in 4 tests being run (including both blockchain test and blockchain engine test combinations).
Example run (without repricing marker)
The full benchmark test suite should run, producing 4 tests:
fill -v tests/benchmark/compute/instruction/test_account_query.py::test_selfbalance --gas-benchmark-values 10 -m benchmark --clean
fill -v tests/benchmark/compute/instruction/test_account_query.py::test_selfbalance --fixed-opcode-count 1 -m benchmark --cleanWith the repricing marker
Only the cases with contract_balance = 0 should run, so it should produce 2 tests:
fill -v tests/benchmark/compute/instruction/test_account_query.py::test_selfbalance --gas-benchmark-values 10 -m repricing --clean
fill -v tests/benchmark/compute/instruction/test_account_query.py::test_selfbalance --fixed-opcode-count 1 -m repricing --cleanEdit:
Regarding the check: if no marker is present, the test should still run when using the -m benchmark flag, but it should be ignored when using the -m repricing flag.
Still take test_selfbalance as example, if you remove the repricing marker, both of this should still be able to run
fill -v tests/benchmark/compute/instruction/test_account_query.py::test_selfbalance --gas-benchmark-values 10 -m benchmark --clean
fill -v tests/benchmark/compute/instruction/test_account_query.py::test_selfbalance --fixed-opcode-count 1 -m benchmark --clean
packages/testing/src/execution_testing/cli/pytest_commands/plugins/shared/benchmarking.py
Show resolved
Hide resolved
packages/testing/src/execution_testing/cli/pytest_commands/plugins/shared/benchmarking.py
Outdated
Show resolved
Hide resolved
packages/testing/src/execution_testing/cli/pytest_commands/plugins/shared/benchmarking.py
Show resolved
Hide resolved
packages/testing/src/execution_testing/cli/pytest_commands/plugins/shared/benchmarking.py
Outdated
Show resolved
Hide resolved
|
Thanks for this! I've tried to address all you comments. Let me know if I missed anything: 1. Fix repricing filter to work with both benchmark options (
2. Allow fixed-opcode-count for all benchmark tests (
3. Warn when config file missing (
4. Update test to match new help text (
5. Remove unnecessary generic_visit call in parser (
|
LouisTsai-Csie
left a comment
There was a problem hiding this comment.
LGTM! Huge thanks for the effort!
…rios (ethereum#1790) * enhance(test-benchmark): use config file for fixed opcode count scenarios * chore(test-benchmark): update help messages for fixed opcode count and gas bench values * chore(test-benchmark): fix repricing filter to work with both benchmark options * chore(test-benchmark): allow fixed-opcode-count for all benchmark tests * chore(test-benchmark): warn when config file missing for fixed-opcode-count * chore(test-benchmark): update test to match new help text * chore(test-benchmark): remove unnecessary generic_visit call in parser * chore(test-benchmark): format test file
…rios (ethereum#1790) * enhance(test-benchmark): use config file for fixed opcode count scenarios * chore(test-benchmark): update help messages for fixed opcode count and gas bench values * chore(test-benchmark): fix repricing filter to work with both benchmark options * chore(test-benchmark): allow fixed-opcode-count for all benchmark tests * chore(test-benchmark): warn when config file missing for fixed-opcode-count * chore(test-benchmark): update test to match new help text * chore(test-benchmark): remove unnecessary generic_visit call in parser * chore(test-benchmark): format test file
…rios (ethereum#1790) * enhance(test-benchmark): use config file for fixed opcode count scenarios * chore(test-benchmark): update help messages for fixed opcode count and gas bench values * chore(test-benchmark): fix repricing filter to work with both benchmark options * chore(test-benchmark): allow fixed-opcode-count for all benchmark tests * chore(test-benchmark): warn when config file missing for fixed-opcode-count * chore(test-benchmark): update test to match new help text * chore(test-benchmark): remove unnecessary generic_visit call in parser * chore(test-benchmark): format test file
🗒️ Description
This PR adds a CLI tool
benchmark_parserto automatically scan benchmark tests and generate a configuration file.fixed_opcode_counts.jsonfor the--fixed-opcode-countfeature from #1747.Key Changes
uv run benchmark_parsertests/benchmark/for tests with@pytest.mark.repricingmarker@pytest.mark.parametrizedecorators.fixed_opcode_counts.jsonat repo root with opcode counts mapping--checkmode for CI validation:uv run benchmark_parser --check.fixed_opcode_counts.jsontest_arithmetic.pyfor consistencyUsage
uv run benchmark_parser.fixed_opcode_counts.json:{ "scenario_configs": { "test_codecopy.*": [ 1 ], ... }Fill works correctly and I tested execute remote on Hoodi with 1K opcode count set, the latest txs: https://hoodi.etherscan.io/address/0x83fd666bfb2b345f932c3e4e04b6d85e5ed3568d
Future Items
--fixed-opcode-countafter generating the config file.--fixed-opcode-countwithdebug_traceTransactionusingexecute hive.🔗 Related Issues or PRs
#1747
✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.